Harden Cryptor input validation, unify error surface, redact log secrets#253
Conversation
jguz-pubnub
changed the title
jguz-pubnub
changed the title
The catch blocks in AESCBCCryptor.encrypt(data:) and LegacyCryptor.encrypt(data:) returned .decryptionFailure even though the call site is encryption, misdirecting incident triage. Both now return .encryptionFailure, aligning with the stream-encrypt paths. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| /// - levels: The log levels to be included in the logger. Defaults to `.none`. | ||
| /// - writers: The writers to be used for logging. Defaults to the default log writers. | ||
| public init(levels: LogLevel = .all, writers: [LogWriter] = PubNubLogger.defaultLogWriters()) { | ||
| public init(levels: LogLevel = .none, writers: [LogWriter] = PubNubLogger.defaultLogWriters()) { |
There was a problem hiding this comment.
The SDK always creates a PubNubLogger internally with the log level disabled. This change removes the insecure default from the public initializer.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swift`:
- Around line 172-177: The LegacyCryptor decrypt flow is masking all thrown
errors from writeEncodedData as CBCDecrypt.failure, which hides non-crypto I/O
problems. Update the decrypt path in LegacyCryptor so only actual
crypto/decryption failures map to CBCDecrypt.failure, and let output-path or
stream/write errors surface with their original details or a more specific
failure case. Keep the existing success path and the InputStream(url:) handling,
but preserve diagnosable error information in the catch around writeEncodedData.
In `@Sources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swift`:
- Around line 111-122: In CryptoInputStream’s IV handling path, don’t treat a
single short positive InputStream.read(_:maxLength:) result as a fatal parse
failure; keep reading until initializationVectorBuffer is fully populated or the
stream truly ends/errors, and preserve any existing streamError when that
happens. Also update open() so it does not reset _streamStatus back to .open
after an IV parse failure, ensuring a bad IV cannot continue into decryption.
Use the existing CryptoInputStream initialization logic and
_streamStatus/_streamError state to locate the fix.
In `@Tests/PubNubUnitTests/Helpers/CryptoTests.swift`:
- Around line 286-375: The new rejection-path tests only cover AESCBCCryptor,
but LegacyCryptor also needs coverage for the same hardening. Add focused tests
in CryptoTests around LegacyCryptor to exercise the cases where the payload is
shorter than the IV and where there is no ciphertext after the IV, using the
existing decrypt-related test helpers and asserting opaque failure. Keep the
tests aligned with the current AESCBCCryptor cases so the legacy behavior is
protected from regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ed07168-3097-4866-a5de-0ec16a2ce97c
📒 Files selected for processing (16)
Snippets/Configuration/configuration.swiftSnippets/Misc/misc.swiftSources/PubNub/Extensions/String+PubNub.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swiftSources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swiftSources/PubNub/Logging/LogWriter.swiftSources/PubNub/Logging/PubNubLogger.swiftSources/PubNub/PubNub.swiftTests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swiftTests/PubNubUnitTests/PubNubConfigurationTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Validate package managers (CocoaPods)
🧰 Additional context used
📓 Path-based instructions (8)
{Sources/PubNub/**/*.swift,Tests/*Tests/**/*.swift}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the shared Swift coding guidance in
CODING_STANDARDS.mdfor production Swift library code and Swift SDK test code.
Files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swiftSources/PubNub/Logging/LogWriter.swiftTests/PubNubUnitTests/PubNubConfigurationTests.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swiftSources/PubNub/Extensions/String+PubNub.swiftSources/PubNub/Logging/PubNubLogger.swiftSources/PubNub/PubNub.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swiftSources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swift
Tests/PubNubIntegrationTests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Integration tests under
Tests/PubNubIntegrationTests/must use real PubNub API keys and make actual network requests.
Files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swift
⚙️ CodeRabbit configuration file
Tests/PubNubIntegrationTests/**/*.swift: Review this as Swift integration test code.Focus on:
- Validate real end-to-end SDK behavior against PubNub services.
- Use the same module imports that SDK customers would use; do not rely on
@testable import.- Avoid making integration coverage redundant with unit tests when behavior can be validated more cheaply in unit tests.
- Watch for flaky behavior, missing cleanup, and hidden environment dependencies.
Files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swift
Tests/**/*.swift
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Tests/**/*.swift: In Swift SDK test code, test names should clearly describe the scenario and expected outcome.
In Swift SDK test code, prefer a clear Arrange-Act-Assert structure when practical.
In Swift SDK test code, cover edge cases and failure paths introduced by the change.
In Swift SDK test code, keep tests deterministic and isolated; avoid hidden shared state, order dependence, and timing-sensitive assertions.
In Swift SDK test code, prefer focused tests over broad tests that verify many behaviors at once.
In Swift SDK test code, in throwing contexts, mark test methods asthrowsand prefertry/try XCTUnwrap()over optional unwrapping patterns to keep tests flat.
In Swift SDK test code, each test method should create its own mutable state and dependencies locally; do not share them via class-levelvarproperties orsetUp()/tearDown(). Class-levelletconstants for static test data are fine.
In Swift SDK test code, use clearly typed test doubles with consistent prefixes:Mock(verifies interactions),Stub(returns canned data),NoOp(null object),Expector(spy with XCTestExpectation).
In Swift SDK test code,unownedcaptures are acceptable; they reduce unwrapping noise and surface lifecycle bugs immediately.
Files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swiftTests/PubNubUnitTests/PubNubConfigurationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdThis file provides guidance to AI coding tools working in this repository.
Scope
This repository contains the official PubNub Swift SDK. The main supported public SDK surface is
PubNubSDK.Prefer guidance in this file over assumptions from source layout alone. If repository structure and this file disagree, update this file as part of the change.
Public Surface
- The primary public entry point is
PubNubinSources/PubNub/PubNub.swift.- Client configuration is provided through
PubNubConfiguration.- The Swift Package Manager product imported by clients is
PubNubSDK.Repository Layout
Sources/PubNub/— main SDK implementationTests/PubNubUnitTests/— primary Swift unit testsTests/PubNubIntegrationTests/— integration testsTests/PubNubContractTests/— contract and acceptance testsExamples/— sample Xcode applicationsSnippets/— documentation code snippets organized by API areaDocumentation/— guides and migration docsfastlane/— CI and release automationPubNub.xcodeproj/PubNub.xcworkspace— Xcode project and workspaceCoding Standards
Follow the shared Swift coding guidance in
CODING_STANDARDS.md. Treat that file as the source of truth for production Swift library code and Swift SDK test code standards.Dependencies
- The SDK has zero external production dependencies. Do not add any.
- Distribution is supported via SPM (primary), CocoaPods, and Carthage.
- The only test dependency is Cucumberish (CocoaPods, for contract tests).
- Platform minimums: iOS 12+, macOS 10.15+, tvOS 12+, watchOS 4+, visionOS 1+. Swift 5.9+.
Architecture Notes
Networking
- Routers in
Sources/PubNub/Networking/Routers/buildURLRequests,HTTPSessionexecutes them, and response decoders inNetworking/Response/handle parsing.- Retry logic lives in
Request.Event Engine
Sources/PubNub/EventEngine/contains the shared state-machine infrastructure.- Subscribe impl...
Files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swiftSources/PubNub/Logging/LogWriter.swiftSnippets/Misc/misc.swiftTests/PubNubUnitTests/PubNubConfigurationTests.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swiftSources/PubNub/Extensions/String+PubNub.swiftSources/PubNub/Logging/PubNubLogger.swiftSources/PubNub/PubNub.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSnippets/Configuration/configuration.swiftSources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swiftSources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swift
Sources/**/*.swift
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Sources/**/*.swift: Prefer value semantics and immutability in production Swift library code; useletinstead ofvarunless mutation is required.
In production Swift library code, minimize access levels: default toprivateorinternal; expose aspubliconly what SDK consumers explicitly need, and do not widen access without justification.
In production Swift library code, avoid force unwraps and other crash-prone patterns.
In production Swift library code, prefer clear, idiomatic Swift naming and small, focused types and functions over overly clever abstractions.
In production Swift library code, preserve useful error information; do not ignore failures or replace specific errors with vague ones.
Files:
Sources/PubNub/Logging/LogWriter.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swiftSources/PubNub/Extensions/String+PubNub.swiftSources/PubNub/Logging/PubNubLogger.swiftSources/PubNub/PubNub.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swiftSources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swift
⚙️ CodeRabbit configuration file
Sources/**/*.swift: Review focus for all production source:
- Flag likely retain cycles, leaked observers, and long-lived captured references.
- Watch for race conditions and shared mutable state, especially around callbacks, queues, async work, and listener management.
- Flag duplicated logic, dead code, and abstractions that add complexity without a real boundary or testability benefit.
- Watch for hot-path inefficiencies such as repeated allocations, avoidable copying, and unnecessary collection transformations.
Files:
Sources/PubNub/Logging/LogWriter.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swiftSources/PubNub/Extensions/String+PubNub.swiftSources/PubNub/Logging/PubNubLogger.swiftSources/PubNub/PubNub.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swiftSources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swift
Snippets/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Keep
// snippet.<id>and// snippet.endmarkers intact inSnippets/{Area}/, and add snippets for new public API.
Files:
Snippets/Misc/misc.swiftSnippets/Configuration/configuration.swift
⚙️ CodeRabbit configuration file
Snippets/**/*.swift: Review this as documentation snippet code intended for SDK users.Focus on:
- Use current public SDK APIs and recommended usage patterns.
- Keep examples clear, minimal, and easy for users to adapt.
- Keep public-facing comments accurate and aligned with current released behavior.
- Do not introduce internal-only, unreleased, or misleading usage patterns.
- Preserve
// snippet.<id>and// snippet.endmarkers required by documentation tooling.
Files:
Snippets/Misc/misc.swiftSnippets/Configuration/configuration.swift
Tests/PubNubUnitTests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Tests/PubNubUnitTests/**/*.swift: InTests/PubNubUnitTests/, class-levelletconstants and value types are acceptable for static test data, but mutable state and reference-type dependencies must be created locally per test method.
InTests/PubNubUnitTests/, mock all HTTP interactions viaMockURLSession; do not make real network calls.
Files:
Tests/PubNubUnitTests/PubNubConfigurationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
⚙️ CodeRabbit configuration file
Tests/PubNubUnitTests/**/*.swift: Test-specific best practices to check:
- Keep unit tests fast, deterministic, and isolated.
- Mock external interactions instead of relying on real network behavior.
- Prefer tests that verify behavior without depending on incidental implementation details.
- Use assertions that clearly validate the intended behavior and failure mode.
Files:
Tests/PubNubUnitTests/PubNubConfigurationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
Sources/PubNub/{PubNub.swift,APIs/**/*.swift}
⚙️ CodeRabbit configuration file
Sources/PubNub/{PubNub.swift,APIs/**/*.swift}: Review this as public SDK API surface exposed throughPubNub.Focus on:
- Keep API patterns, naming, parameter semantics, and overload design consistent with the existing
PubNubsurface.- Check completion and callback behavior, especially around async results, threading expectations, and error delivery.
- Avoid exposing APIs, types, members, or initializers wider than needed.
- Keep public-facing doc comments accurate and aligned with current behavior, parameters, defaults, and callback semantics.
- Make breaking changes to public API or behavior explicit, justified, and reflected in tests, snippets, and documentation.
Files:
Sources/PubNub/PubNub.swift
🧠 Learnings (2)
📚 Learning: 2026-05-14T12:09:05.831Z
Learnt from: jguz-pubnub
Repo: pubnub/swift PR: 250
File: Tests/PubNubUnitTests/Helpers/ValidatedTests.swift:34-37
Timestamp: 2026-05-14T12:09:05.831Z
Learning: In pubnub/swift Swift unit tests, when a test’s explicit purpose is to verify throwing vs non-throwing behavior, prefer XCTest’s declarative assertions: use `XCTAssertNoThrow { ... }` to assert the code does not throw and `XCTAssertThrowsError { ... }` to assert it throws. Avoid replacing these with the `throws` + bare `try` pattern for the same check, since it reduces the test’s declarative intent even if other coding standards suggest flat throwing tests in different contexts.
Applied to files:
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swiftTests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swiftTests/PubNubUnitTests/PubNubConfigurationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
📚 Learning: 2026-05-14T12:12:11.923Z
Learnt from: jguz-pubnub
Repo: pubnub/swift PR: 250
File: Tests/PubNubUnitTests/Networking/Routers/ObjectsUserRouterTests.swift:0-0
Timestamp: 2026-05-14T12:12:11.923Z
Learning: In PubNub Swift unit tests under Tests/PubNubUnitTests/**/*.swift, prefer `[unowned]` captures in closures over `[weak]` captures. Do not flag `[unowned]` captures in test code as potential issues during review, since they are acceptable/preferred in this test suite and help expose object-lifecycle mistakes immediately without adding optional-unwrapping noise.
Applied to files:
Tests/PubNubUnitTests/PubNubConfigurationTests.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
🔇 Additional comments (15)
Sources/PubNub/Extensions/String+PubNub.swift (1)
74-91: LGTM!Sources/PubNub/PubNub.swift (1)
79-80: LGTM!Sources/PubNub/Helpers/Crypto/CryptoModule.swift (1)
588-589: LGTM!Also applies to: 646-646, 685-685
Sources/PubNub/Logging/LogWriter.swift (1)
48-50: LGTM!Sources/PubNub/Logging/PubNubLogger.swift (1)
89-95: LGTM!Sources/PubNub/Helpers/Crypto/Miscellaneous/Data+CommonCrypto.swift (1)
14-27: LGTM!Sources/PubNub/Helpers/Crypto/Cryptors/AESCBCCryptor.swift (1)
19-23: LGTM!Also applies to: 59-61, 65-72, 86-86, 126-134, 155-160
Snippets/Configuration/configuration.swift (1)
41-41: LGTM!Also applies to: 55-55
Snippets/Misc/misc.swift (1)
32-32: LGTM!Also applies to: 44-44
Tests/PubNubIntegrationTests/PublishEndpointIntegrationTests.swift (1)
214-220: LGTM!Tests/PubNubUnitTests/PubNubConfigurationTests.swift (1)
69-75: LGTM!Sources/PubNub/Helpers/Crypto/Cryptors/LegacyCryptor.swift (1)
71-71: LGTM!Also applies to: 83-110
Tests/PubNubIntegrationTests/FilesEndpointIntegrationTests.swift (1)
226-226: LGTM!Tests/PubNubIntegrationTests/HistoryEndpointIntegrationTests.swift (1)
256-256: LGTM!Tests/PubNubUnitTests/Helpers/CryptoTests.swift (1)
12-12: LGTM!Also applies to: 77-77, 114-116
jguz-pubnub
force-pushed
the
swift-sec-fixes
branch
from
18ab82a to
9cae998
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/PubNub/Helpers/Crypto/CryptoModule.swift`:
- Around line 249-252: The decryption failure in performDataDecryption(data:) is
using a copy-pasted message that incorrectly says “Could not decrypt
InputStream.” Update the additional error detail in CryptoModule.swift so it
accurately describes the data-based path, and keep the message aligned with the
surrounding decryption logic and PubNubError(.decryptionFailure) construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cfa9f58c-a4a7-4e7d-bcea-51236331265f
📒 Files selected for processing (4)
Sources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Validate package managers (Swift Package Manager)
🧰 Additional context used
📓 Path-based instructions (5)
{Sources/PubNub/**/*.swift,Tests/*Tests/**/*.swift}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the shared Swift coding guidance in
CODING_STANDARDS.mdfor production Swift library code and Swift SDK test code.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Errors/PubNubError.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
Sources/**/*.swift
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Sources/**/*.swift: Prefer value semantics and immutability in production Swift library code; useletinstead ofvarunless mutation is required.
In production Swift library code, minimize access levels: default toprivateorinternal; expose aspubliconly what SDK consumers explicitly need, and do not widen access without justification.
In production Swift library code, avoid force unwraps and other crash-prone patterns.
In production Swift library code, prefer clear, idiomatic Swift naming and small, focused types and functions over overly clever abstractions.
In production Swift library code, preserve useful error information; do not ignore failures or replace specific errors with vague ones.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
⚙️ CodeRabbit configuration file
Sources/**/*.swift: Review focus for all production source:
- Flag likely retain cycles, leaked observers, and long-lived captured references.
- Watch for race conditions and shared mutable state, especially around callbacks, queues, async work, and listener management.
- Flag duplicated logic, dead code, and abstractions that add complexity without a real boundary or testability benefit.
- Watch for hot-path inefficiencies such as repeated allocations, avoidable copying, and unnecessary collection transformations.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdThis file provides guidance to AI coding tools working in this repository.
Scope
This repository contains the official PubNub Swift SDK. The main supported public SDK surface is
PubNubSDK.Prefer guidance in this file over assumptions from source layout alone. If repository structure and this file disagree, update this file as part of the change.
Public Surface
- The primary public entry point is
PubNubinSources/PubNub/PubNub.swift.- Client configuration is provided through
PubNubConfiguration.- The Swift Package Manager product imported by clients is
PubNubSDK.Repository Layout
Sources/PubNub/— main SDK implementationTests/PubNubUnitTests/— primary Swift unit testsTests/PubNubIntegrationTests/— integration testsTests/PubNubContractTests/— contract and acceptance testsExamples/— sample Xcode applicationsSnippets/— documentation code snippets organized by API areaDocumentation/— guides and migration docsfastlane/— CI and release automationPubNub.xcodeproj/PubNub.xcworkspace— Xcode project and workspaceCoding Standards
Follow the shared Swift coding guidance in
CODING_STANDARDS.md. Treat that file as the source of truth for production Swift library code and Swift SDK test code standards.Dependencies
- The SDK has zero external production dependencies. Do not add any.
- Distribution is supported via SPM (primary), CocoaPods, and Carthage.
- The only test dependency is Cucumberish (CocoaPods, for contract tests).
- Platform minimums: iOS 12+, macOS 10.15+, tvOS 12+, watchOS 4+, visionOS 1+. Swift 5.9+.
Architecture Notes
Networking
- Routers in
Sources/PubNub/Networking/Routers/buildURLRequests,HTTPSessionexecutes them, and response decoders inNetworking/Response/handle parsing.- Retry logic lives in
Request.Event Engine
Sources/PubNub/EventEngine/contains the shared state-machine infrastructure.- Subscribe impl...
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Errors/PubNubError.swiftTests/PubNubUnitTests/Helpers/CryptoTests.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
Tests/PubNubUnitTests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Tests/PubNubUnitTests/**/*.swift: InTests/PubNubUnitTests/, class-levelletconstants and value types are acceptable for static test data, but mutable state and reference-type dependencies must be created locally per test method.
InTests/PubNubUnitTests/, mock all HTTP interactions viaMockURLSession; do not make real network calls.
Files:
Tests/PubNubUnitTests/Helpers/CryptoTests.swift
⚙️ CodeRabbit configuration file
Tests/PubNubUnitTests/**/*.swift: Test-specific best practices to check:
- Keep unit tests fast, deterministic, and isolated.
- Mock external interactions instead of relying on real network behavior.
- Prefer tests that verify behavior without depending on incidental implementation details.
- Use assertions that clearly validate the intended behavior and failure mode.
Files:
Tests/PubNubUnitTests/Helpers/CryptoTests.swift
Tests/**/*.swift
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Tests/**/*.swift: In Swift SDK test code, test names should clearly describe the scenario and expected outcome.
In Swift SDK test code, prefer a clear Arrange-Act-Assert structure when practical.
In Swift SDK test code, cover edge cases and failure paths introduced by the change.
In Swift SDK test code, keep tests deterministic and isolated; avoid hidden shared state, order dependence, and timing-sensitive assertions.
In Swift SDK test code, prefer focused tests over broad tests that verify many behaviors at once.
In Swift SDK test code, in throwing contexts, mark test methods asthrowsand prefertry/try XCTUnwrap()over optional unwrapping patterns to keep tests flat.
In Swift SDK test code, each test method should create its own mutable state and dependencies locally; do not share them via class-levelvarproperties orsetUp()/tearDown(). Class-levelletconstants for static test data are fine.
In Swift SDK test code, use clearly typed test doubles with consistent prefixes:Mock(verifies interactions),Stub(returns canned data),NoOp(null object),Expector(spy with XCTestExpectation).
In Swift SDK test code,unownedcaptures are acceptable; they reduce unwrapping noise and surface lifecycle bugs immediately.
Files:
Tests/PubNubUnitTests/Helpers/CryptoTests.swift
🧠 Learnings (2)
📚 Learning: 2026-05-14T12:09:05.831Z
Learnt from: jguz-pubnub
Repo: pubnub/swift PR: 250
File: Tests/PubNubUnitTests/Helpers/ValidatedTests.swift:34-37
Timestamp: 2026-05-14T12:09:05.831Z
Learning: In pubnub/swift Swift unit tests, when a test’s explicit purpose is to verify throwing vs non-throwing behavior, prefer XCTest’s declarative assertions: use `XCTAssertNoThrow { ... }` to assert the code does not throw and `XCTAssertThrowsError { ... }` to assert it throws. Avoid replacing these with the `throws` + bare `try` pattern for the same check, since it reduces the test’s declarative intent even if other coding standards suggest flat throwing tests in different contexts.
Applied to files:
Tests/PubNubUnitTests/Helpers/CryptoTests.swift
📚 Learning: 2026-05-14T12:12:11.923Z
Learnt from: jguz-pubnub
Repo: pubnub/swift PR: 250
File: Tests/PubNubUnitTests/Networking/Routers/ObjectsUserRouterTests.swift:0-0
Timestamp: 2026-05-14T12:12:11.923Z
Learning: In PubNub Swift unit tests under Tests/PubNubUnitTests/**/*.swift, prefer `[unowned]` captures in closures over `[weak]` captures. Do not flag `[unowned]` captures in test code as potential issues during review, since they are acceptable/preferred in this test suite and help expose object-lifecycle mistakes immediately without adding optional-unwrapping noise.
Applied to files:
Tests/PubNubUnitTests/Helpers/CryptoTests.swift
🔇 Additional comments (6)
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swift (1)
109-113: LGTM!Also applies to: 126-126
Tests/PubNubUnitTests/Helpers/CryptoTests.swift (3)
12-12: LGTM!Also applies to: 25-25, 49-49, 78-100
113-121: LGTM!Also applies to: 152-161, 186-194
292-413: LGTM!Sources/PubNub/Errors/PubNubError.swift (1)
22-30: LGTM!Sources/PubNub/Helpers/Crypto/CryptoModule.swift (1)
641-649: LGTM!Also applies to: 680-688
jguz-pubnub
force-pushed
the
swift-sec-fixes
branch
from
9cae998 to
beca16e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swift`:
- Around line 238-251: Avoid the force unwrap in CryptoInputStream.readExactly;
ptr.baseAddress! can crash if the buffer pointer is nil. Update
readExactly(from:into:length:) to safely handle a nil base address from
withUnsafeMutableBufferPointer, returning false or otherwise exiting cleanly
instead of force-unwrapping while preserving the existing read loop behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90bf41b6-15ea-4f8e-943c-b082af43e5d1
📒 Files selected for processing (4)
Sources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swiftSources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swift
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Integration tests
🧰 Additional context used
📓 Path-based instructions (3)
{Sources/PubNub/**/*.swift,Tests/*Tests/**/*.swift}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the shared Swift coding guidance in
CODING_STANDARDS.mdfor production Swift library code and Swift SDK test code.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
Sources/**/*.swift
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Sources/**/*.swift: Prefer value semantics and immutability in production Swift library code; useletinstead ofvarunless mutation is required.
In production Swift library code, minimize access levels: default toprivateorinternal; expose aspubliconly what SDK consumers explicitly need, and do not widen access without justification.
In production Swift library code, avoid force unwraps and other crash-prone patterns.
In production Swift library code, prefer clear, idiomatic Swift naming and small, focused types and functions over overly clever abstractions.
In production Swift library code, preserve useful error information; do not ignore failures or replace specific errors with vague ones.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
⚙️ CodeRabbit configuration file
Sources/**/*.swift: Review focus for all production source:
- Flag likely retain cycles, leaked observers, and long-lived captured references.
- Watch for race conditions and shared mutable state, especially around callbacks, queues, async work, and listener management.
- Flag duplicated logic, dead code, and abstractions that add complexity without a real boundary or testability benefit.
- Watch for hot-path inefficiencies such as repeated allocations, avoidable copying, and unnecessary collection transformations.
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdThis file provides guidance to AI coding tools working in this repository.
Scope
This repository contains the official PubNub Swift SDK. The main supported public SDK surface is
PubNubSDK.Prefer guidance in this file over assumptions from source layout alone. If repository structure and this file disagree, update this file as part of the change.
Public Surface
- The primary public entry point is
PubNubinSources/PubNub/PubNub.swift.- Client configuration is provided through
PubNubConfiguration.- The Swift Package Manager product imported by clients is
PubNubSDK.Repository Layout
Sources/PubNub/— main SDK implementationTests/PubNubUnitTests/— primary Swift unit testsTests/PubNubIntegrationTests/— integration testsTests/PubNubContractTests/— contract and acceptance testsExamples/— sample Xcode applicationsSnippets/— documentation code snippets organized by API areaDocumentation/— guides and migration docsfastlane/— CI and release automationPubNub.xcodeproj/PubNub.xcworkspace— Xcode project and workspaceCoding Standards
Follow the shared Swift coding guidance in
CODING_STANDARDS.md. Treat that file as the source of truth for production Swift library code and Swift SDK test code standards.Dependencies
- The SDK has zero external production dependencies. Do not add any.
- Distribution is supported via SPM (primary), CocoaPods, and Carthage.
- The only test dependency is Cucumberish (CocoaPods, for contract tests).
- Platform minimums: iOS 12+, macOS 10.15+, tvOS 12+, watchOS 4+, visionOS 1+. Swift 5.9+.
Architecture Notes
Networking
- Routers in
Sources/PubNub/Networking/Routers/buildURLRequests,HTTPSessionexecutes them, and response decoders inNetworking/Response/handle parsing.- Retry logic lives in
Request.Event Engine
Sources/PubNub/EventEngine/contains the shared state-machine infrastructure.- Subscribe impl...
Files:
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swiftSources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swiftSources/PubNub/Errors/PubNubError.swiftSources/PubNub/Helpers/Crypto/CryptoModule.swift
🪛 SwiftLint (0.65.0)
Sources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swift
[Warning] 243-243: Force unwrapping should be avoided
(force_unwrapping)
🔇 Additional comments (4)
Sources/PubNub/Helpers/Crypto/Header/CryptorHeader.swift (1)
107-127: LGTM!Sources/PubNub/Helpers/Crypto/Miscellaneous/CryptoInputStream.swift (1)
105-121: LGTM! IV exact-read hardening correctly turns short/truncated reads into a stream error instead of silently proceeding with a partial IV.Sources/PubNub/Helpers/Crypto/CryptoModule.swift (1)
181-256: LGTM! Error-message standardization is consistent per path, and the previously flagged copy-pasted "InputStream" message inperformDataDecryption(line 252) is now correctly fixed to "Could not decrypt Data". Plaintext removal from debug logs matches the stated redaction goal.Also applies to: 441-503, 504-572, 593-595, 641-648, 680-730
Sources/PubNub/Errors/PubNubError.swift (1)
22-30: LGTM!
Loop until a full IV block is read so legitimate file/network streams that return short reads are not wrongly rejected, while truncated payloads still fail.
jguz-pubnub
force-pushed
the
swift-sec-fixes
branch
from
c4548df to
3f32883
Compare
jguz-pubnub
changed the title
jguz-pubnub
changed the title
|
@pubnub-release-bot release |
jguz-pubnub
force-pushed
the
swift-sec-fixes
branch
from
9900c41 to
61ceacc
Compare
jguz-pubnub
force-pushed
the
swift-sec-fixes
branch
from
61ceacc to
32dbcfd
Compare
jguz-pubnub
changed the title
|
🚀 Release successfully completed 🚀 |
3 participants
fix: validate IV length and ciphertext block-alignment before decrypting
fix: return a uniform .decryptionFailure that never exposes the underlying CommonCrypto status
fix: guard LegacyCryptor random-IV path against payloads too short to contain an IV
fix: map encrypt-path errors to .encryptionFailure instead of .decryptionFailure
fix: redact authKey and authToken in instance logs instead of emitting them in cleartext
fix: use .none as the default log level for the PubNubLogger public initializer to avoid an insecure default
docs: warn that verbose log levels may expose sensitive data and recommend long, high-entropy cipher keys
fix: make message action add/remove fire completion only once on error
refactor: make every PubNub instance manage its own presence-state container